Conversation
…n (4 review threads absorbed) Four substantive post-merge review findings on B-0142: 1. Existing invalidArg/exception-based code in src/Core/** — programmer-error preconditions historically use exceptions in F# idiom; CLAUDE.md Result-over-exception applies to user-visible failures. Scope refinement: B-0142 targets recoverable user-facing failures (Result-flow); programmer-error preconditions stay as invalidArg until a separate decision migrates them. NO blanket-ban exceptions. 2. Plain-returning APIs (bool/Stream/ValueTask) — contract primitives returning Result aren't drop-in. Scope refinement: two-mode primitive — requires_result() (Result-returning) vs requires_assert() (assertion-only). 3. TreatWarningsAsErrors=true makes warning vs error a false dichotomy. Scope refinement: contract-layer severity field; build-gate disposition is uniform (any reaching build = break); per-contract- class CI gating is separate future design question. 4. Dangling reference: memory/parallelism-scaling-ladder memo replaced with full filename for grep-ability. New "Migration concerns + scope clarification" section absorbs the findings. Open-design-questions updated with build-gate disposition question replacing the warning-vs-error false dichotomy.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 849ee4edd9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Adds a new “Migration concerns + scope clarification” section to backlog row B-0142 to absorb post-merge review feedback and narrow the intended contract-mechanization scope without forcing repo-wide signature churn.
Changes:
- Replaces a vague memo reference with a full
memory/...filename for grep-ability. - Introduces a new section documenting migration concerns and refining scope around
invalidArg, Result-flow, plain-returning APIs, andTreatWarningsAsErrors. - Updates the open design questions to reflect the clarified build-gate framing.
… no-throw rule explicitly to recoverable-user-facing-failures + Stream type precision
AceHack
deleted the
backlog/B-0142-followup-migration-concerns-warnings-as-errors
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four substantive post-merge review findings on B-0142 absorbed via new "Migration concerns + scope clarification" section.
Findings absorbed
Existing `invalidArg` / exception-based code in `src/Core/`** (line 24) — programmer-error preconditions use F# idiom (`invalidArg`); CLAUDE.md Result-over-exception applies to user-visible failures. Scope refinement: B-0142 targets recoverable user-facing failures (Result-flow); programmer-error preconditions stay as `invalidArg` until separate decision migrates them. No blanket-ban exceptions.
Plain-returning APIs (`bool`, `Stream`, `ValueTask`) (line 55) — contract primitives returning `Result` aren't drop-in. Scope refinement: two-mode primitive — `requires_result()` (Result-returning) vs `requires_assert()` (assertion-only).
`TreatWarningsAsErrors=true` makes warning vs error a false dichotomy (line 91). Scope refinement: contract-layer severity field; build-gate disposition is uniform (any reaching build = break); per-contract-class CI gating is separate future design question.
Dangling memo reference (line 75) — `memory/parallelism-scaling-ladder memo` replaced with full filename for grep-ability.
Why this matters
These are all real compatibility concerns that affect future B-0142 implementation. The original row's spec admitted scope-creep that conflicted with existing F# idiom. Refining the scope keeps B-0142 viable as durable backlog substrate without forcing repo-wide signature churn.
Test plan